Skip to content

fix(audio-studio/ios): make AAC trims honour the requested rate and depth - #461

Merged
abretonc7s merged 11 commits into
mainfrom
fix/ios-trim-aac-bitdepth-451
Aug 22, 2026
Merged

fix(audio-studio/ios): make AAC trims honour the requested rate and depth#461
abretonc7s merged 11 commits into
mainfrom
fix/ios-trim-aac-bitdepth-451

Conversation

@abretonc7s

Copy link
Copy Markdown
Collaborator

Fixes #451.

Reproduced, fixed, validated on device

case before after (on simulator)
1000ms trim → AAC @ 22.05kHz ~2000ms 1114ms (AAC priming), rate 22050
1000ms trim → WAV bitDepth 32 ignored (fast path) bitDepth 32, 1000ms
plain single / keepRanges 5000/4000ms unchanged

The fixes

Rate: the AAC path wrote source-format buffers to a target-rate writer. Now converts against the writer's resolved format — the same treatment the WAV path received in #450: ratio-sized output buffer, single-shot input, downmix on channel reduction, throw instead of skip, writer scoped before read-back.

Bitrate: fixing the rate exposed a pre-existing defect — the encoder rejects bitrates its profile can't serve at the target rate. Measured: 96kbps+ fails at 22.05kHz mono (error 560226676); no formula predicts the ceiling — my first guess of 8 bits/sample/channel was wrong, the real limit there is ~3. So the writer opens with the requested bitrate and reopens without one when refused, since omitting the key always succeeds.

Depth: omitted bitDepth forced 16 during any conversion, contrary to the preserve-input contract; a depth-only request took the fast path and was ignored. Default is now the input's own depth, depth changes leave the fast path, no result hardcodes 16.

Infrastructure

Adds testTrimAudioWith(options) to the agentic bridge — the existing testTrimAudio is fixed-options, so trim changes couldn't be validated on device with the failing configurations.

Suites

iOS 83 passed, typecheck clean, app BUILD SUCCEEDED (fresh install; lastUpdateTime checked).

abretonc7s added 5 commits August 21, 2026 15:06
…epth

The AAC path wrote source-format buffers to a target-rate writer, violating
AVAudioFile's format-match contract and mis-timing the output. Probed at a
44.1kHz source: one second came back as 2.0000s at 22.05kHz and 0.9187s at
48kHz. It now converts against the writer's resolved format — the same
treatment the WAV path received in #450 — with the same ratio-sized output
buffer, single-shot input, downmix on channel reduction, and errors instead of
silent skips. The writer is scoped so the read-back sees the finished file.

Fixing the rate exposed a pre-existing bitrate defect: the encoder rejects
combinations its profile cannot serve — measured, 96kbps and up fail at
22.05kHz mono with error 560226676, and no formula predicts the ceiling (my
first guess of 8 bits/sample/channel was wrong; the real limit there is ~3).
So the writer is opened with the requested bitrate and reopened without one
when the encoder refuses, honouring the request where possible instead of
failing a trim the platform can serve.

Bit depth: an omitted bitDepth forced 16 during any rate or channel change,
contrary to the contract that the input format is preserved, and a
bitDepth-only request took the fast path and was ignored. The default is now
the input's own depth, depth changes leave the fast path, and no result
hardcodes 16.

Validated on the iOS simulator, per the hard rule:
- 1000ms trim to AAC@22050: durationMs 1114 (AAC priming), rate 22050 —
  previously ~2000ms
- 1000ms trim to WAV bitDepth 32: durationMs 1000, bitDepth 32 — previously
  ignored
- plain single/keepRanges trims unchanged at 5000ms/4000ms

Adds testTrimAudioWith(options) to the agentic bridge so trim changes can be
validated on device with arbitrary options.
…olving them

Three review blockers, all reproduced on device before fixing.

The AAC path checked only the writer's resolved format, so a rate the platform
cannot serve came back as something else: an AAC writer turns 1Hz into 8kHz and
that conversion succeeds. The requested format is now checked first, matching
the WAV path. On device, sampleRate: 1 rejects where it previously would have
returned 8kHz audio.

An explicitly requested bitrate the encoder refuses was silently replaced with
its default — the result carries no effective bitrate and a debug log is not
caller-visible. An explicit request now errors; only the library's own 128000
default gives way, which makes the encoder default intentional rather than a
swallowed failure. On device: explicit 128k at 22.05kHz rejects, the same trim
without a bitrate succeeds at 22050.

Bit depth clamped the input before comparing it, so a 24-bit source asked for
16-bit compared equal, took the fast path, kept 24 bits and reported 16. The
raw depth now drives the comparison while a separate value supplies the
writable default, so unexpressible depths still default sensibly without
hiding a real conversion.

Device validation (iOS simulator, fresh install):
- aac@1Hz: REJECTED (was: 8kHz output)
- aac@22050 with explicit 128k: REJECTED; without: 1115ms @ 22050
- wav bitDepth 32: 1000ms, depth 32
- plain single/keepRanges: 5000ms/4000ms, unchanged
…substitute

The requested-format check proved a PCM conversion existed, not that the
encoder would use the rate asked for. Probed: the AAC writer resolves 1Hz and
7999Hz to 8000Hz, and 384000Hz to 192000Hz — all of which passed the PCM check
and would have returned audio at a rate the caller never requested.

The writer's resolved rate is now compared against the request and a
substitution refused. On device: aac@1 and aac@384000 reject, while
aac@8000/22050/48000 succeed at exactly the rate requested.

Changelog corrected — it still described bitrate fallback as the behaviour when
an explicit request now errors.
Round-2 review found three defects, each reproduced with an AVFoundation probe.

The fast path claimed to preserve the input and was the one place not doing it.
It wrote `inputFormat.settings`, where inputFormat is processingFormat — float32
for every PCM WAV. Probe: a genuine 16-bit source produced a 32-bit float output
while the result reported bitDepth 16. Now writes fileFormat.settings.

The [16, 32] writable-depth allowlist was wrong. Probed each depth against
AVAudioFile: 8, 16, 24 and 32 all round-trip at the requested depth. The
allowlist silently downconverted 8- and 24-bit sources to 16 with nothing
requested, contradicting the documented preserve-input contract.

The AAC conversion loop discarded the converter status and accepted zero output
frames. A 44.1kHz-to-8kHz conversion of 1-4 input frames returns .endOfStream
with no error and zero frames; the loop wrote that buffer and counted the input
frames, so the reported duration described audio the file does not contain. Now
checks the status, skips empty buffers, counts written frames, and refuses to
promote a file that received none.

Two more found while fixing those, unrelated to #451:

- `?? 16 / 8` parsed as `?? (16 / 8)`, so with the bit-depth key present
  bytesPerSample was 16 rather than 2 and every byte offset in the preview
  loop came out eight times too large.
- In the AAC block, totalFrames started at zero and grew as work completed while
  cumulativeFrames was never incremented there at all — progress was a stale
  outer value divided by a moving total. The denominator is now precomputed and
  input consumed is tracked separately from frames written.

Coverage: AudioProcessor.swift cannot join the SwiftPM test target (it pulls in
the Expo module graph), which is why every one of these shipped untested. The
format decisions are extracted into TrimFormatResolution and used by
AudioProcessor, with 8 tests over the depth and fast-path rules.

91 iOS tests pass, up from 83.
… one converter helper

Round-3 review found two defects. The first is one I should have caught: the
production target did not compile.

`writtenFrames` was declared inside the AAC autoreleasepool closure and read by
the empty-output guard after it — `cannot find 'writtenFrames' in scope`. The
declaration is now outside the closure. I reported "91 iOS tests pass" as
evidence last round, and that was worthless here: Package.swift does not list
AudioProcessor.swift (it pulls in the Expo module graph), so the suite never
compiled the file I changed. Verified this time by building the app target:
BUILD SUCCEEDED with the file compiled for real.

The WAV re-encode path still promoted empty output as success. It had the same
zero-frame defect fixed on the AAC path last round — discarded converter status,
wrote zero-frame buffers, promoted unconditionally. The reviewer's probe produced
a promoted 4096-byte WAV with length 0 from a 44.1kHz-to-8kHz conversion of 1 to
4 frames.

Rather than fix it twice, both loops now call one `convertOneBuffer` helper that
owns the supply-once callback, the error check, the status check and the frame
count. The duplication is what let the two paths drift in the first place — one
got the guard, the other did not — which was also the reviewer's nit.

Also required a `pod install`: the podspec glob covers TrimFormatResolution.swift
but the existing pod project predated it, so the build reported "Build input file
cannot be found" until the project was regenerated.

91 iOS tests pass, and the app target builds.
@abretonc7s

Copy link
Copy Markdown
Collaborator Author

Device validation (iOS Simulator, iPhone 17 Pro Max)

Built and installed the app target from this branch, then exercised trim end to end.

Recorded a source file:

uri: .../09B13F1B-97EB-4050-9B41-4815D300CC75.wav
size: 255824, dur: 3016

Trimmed it via __AGENTIC__.testTrimAudio() — both outputs succeeded:

single:     durationMs 5000, size 164096
keepRanges: durationMs 4000, size 132096

The check that matters, run against the actual files with afinfo:

Data format:      1 ch,  16000 Hz, Int16
source bit depth: I16

Both outputs. Genuine 16-bit integer WAVs.

For contrast, what the pre-fix fast path produced — an AVFoundation probe writing processingFormat.settings, which is what the old code did:

SOURCE processingFormat mBitsPerChannel = 32
OUTPUT actual mBitsPerChannel = 32 isFloat = true
BUT trimAudio would report bitDepth = 16

A 32-bit float file reported as 16-bit. That is the defect, and it is gone.

Build verification

BUILD SUCCEEDED for the AudioDevPlayground target with AudioProcessor.swift compiled. This is what caught the writtenFrames scope error — yarn test:ios cannot, because Package.swift does not list that file. I quoted "91 tests pass" as evidence last round and it proved nothing about the file I had changed.

91 SwiftPM tests also pass, covering the extracted TrimFormatResolution rules.

Note: the installed bundle predates testTrimAudioWith, so this used the existing testTrimAudio hook. The AAC-specific paths and the empty-conversion guard are not exercised by it — those remain covered by probe and unit test only.

abretonc7s added 6 commits August 21, 2026 23:16
This branch adds ios/TrimFormatResolution.swift, and the podspec glob publishes
it, so the package now ships 439 files rather than 438. The manifest check
merged in #469 caught exactly that — which is the case it was written for.

Regenerated: one line added, nothing else changed.
…ts tail

Review found multi-range trims produce different audio depending on how the
selection is split. Both re-encode loops built a fresh AVAudioConverter per
range, and convertOneBuffer signalled .endOfStream after each buffer, so the
resampler's filter state and fractional sample position were discarded between
ranges.

Measured at 44.1 to 48kHz, converting 4410 frames:

  as one range                    4800 frames
  as 100 adjacent 44-frame ranges   47 frames

Two changes. The converter is created once, before the loop, in both the WAV and
AAC paths. And convertOneBuffer now reports .noDataNow rather than .endOfStream
when its single buffer is consumed, which keeps the stream open instead of
finalizing the resampler. .endOfStream was the larger half of the problem:
hoisting alone does nothing while every call still ends the stream.

.noDataNow leaves samples buffered when the ranges run out, so both loops now
drain the converter afterwards. That recovers the tail:

  as one range                    4800
  as 100 ranges, after the fix    4789
  ideal                           4800

0.2% short across a hundred splices rather than 99%.

The first version of the drain compiled under `yarn test:ios` and failed the real
build with three "cannot find in scope" errors, because it sat outside the
autoreleasepool holding outputFile and the converter. That suite does not compile
AudioProcessor.swift, which is why the app target is the check that counts here.

91 SwiftPM tests pass and the app target builds with AudioProcessor.swift
compiled.
…ized rate

Three findings from review, the first a direct consequence of my last commit.

Switching to .noDataNow let AVAudioConverter fill the destination entirely from
output it had already queued, returning without ever invoking the callback. The
input buffer went unconsumed while the loop advanced to the next range and
dropped it. Probed on the simulator: ranges [4410, 44] produced 4800 frames
against 4848 expected, and [4410, 44 x 10] produced 4943 against 5279.
convertOneBuffer now converts until the callback has actually taken the input,
writing each output buffer as it appears — one input can yield several, so both
call sites stopped assuming the destination holds all of it.

The AAC writer echoes the requested sample rate while finalization can
substitute a different one. Probed: 8001 to 8000, 22051 to 22050, 44099 and
44101 to 44100, 48001 to 48000. The work file is now reopened and its rate
checked before promotion, so the result cannot report a rate the file does not
have.

The drain broke on zero frames before checking status, so a .error with no
NSError set looked like a clean end of stream and truncated output was promoted
as success. Status is checked first now. A failed buffer allocation there also
ended the flush silently; that is an error rather than a break, since the
alternative is silent truncation.

testEveryProbedDepthIsWritable compared the constant to itself while claiming to
have verified against AVAudioFile. It now writes and reopens a WAV at each of 8,
16, 24 and 32 bits and asserts the depth survives.

91 SwiftPM tests pass and the app target builds with AudioProcessor.swift
compiled. On the simulator, trimAudio produces 5.000s and 4.000s outputs, both
Int16, the second being the multi-range case these fixes are about.
Review: the pre-promotion validation used try?, so a work file that could not be
reopened skipped the rate check entirely and was promoted anyway — an unreadable
output reported as success, which is worse than the substituted-rate case the
check was added for. Reopening now throws with the underlying reason.

Also merged the two `### Fixed` sections that had accumulated under
[Unreleased]. All entries kept; the section order is now Fixed, Added, Changed.

The reviewer's probe confirms the convert loop from the last commit behaves:
ranges [4410, 44] needed eight queued-output writes before the converter took
the second buffer, and finalized correctly.

91 SwiftPM tests pass, the app target builds, and on the simulator trimAudio
still produces a 5.000s Int16 output.
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
5.0% Duplication on New Code (required ≤ 4%)

See analysis details on SonarQube Cloud

@abretonc7s
abretonc7s merged commit 929bb32 into main Aug 22, 2026
3 of 4 checks passed
@abretonc7s
abretonc7s deleted the fix/ios-trim-aac-bitdepth-451 branch August 22, 2026 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS trimAudio: AAC path and bit-depth handling produce wrong output

1 participant